Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

MicroPython: Avoid heap allocations in all C++ modules. #711

Merged
merged 23 commits into from
Mar 17, 2023

Conversation

Gadgetoid
Copy link
Member

As of micropython/micropython@c80e7c1 MicroPython's linker script includes a "greedy" heap, which attempts to use all free system RAM for MicroPython's heap.

Whereas previously we carefully avoided large heap allocations due to the very limited amount of heap available to C++ extensions, we must now avoid all allocations.

This changeset cleans up some uses of std::string which were implicitly allocating strings on heap. It does this by switching to string_view, wrapping the existing byte array and length returned by GET_STR_DATA_LEN instead of copying them into a std::string.

Additionally set_font() performs Hershey font lookups via a new if/else table, since the std::map<std::string, const *font_t> lookup table could not be used without constructing a string and risking a heap allocation.

Finally I have removed the completely ridiculous and unnecessary conversion to/from a std::string in our jpegdec binding's open_file functionality.

To make string_view available, all MicroPython builds needed to be forced to use C++17, this is achieved in their respective CMake files.

In all cases I believe std::string will implicitly cast to std::string_view so our examples will not break, and C++ code has always been set to C++17.

@Gadgetoid
Copy link
Member Author

This is a pre-requisite to bumping all of our MicroPython builds to the latest edge commit and - indeed - keeping ahead of MicroPython changes so we can incorporate Bluetooth when it arrives.

Once merged, relevant commits from https://github.com/pimoroni/badger2040/tree/test/micropython-edge can be moved to pimoroni/badger2040#2 (and the GitHub actions .yml targeted back at the main branch here)

@Gadgetoid Gadgetoid force-pushed the patch-jpegdec-filename branch from dfc696f to c529bd6 Compare March 13, 2023 15:06
@Gadgetoid Gadgetoid force-pushed the patch-jpegdec-filename branch from a16fb8e to 210a06b Compare March 13, 2023 21:15
@Gadgetoid Gadgetoid changed the title Avoid implicit heap allocations when using std::string MicroPython: Avoid heap allocations in all C++ modules. Mar 13, 2023
@Gadgetoid
Copy link
Member Author

Pico RGB Keypad, Pico Scroll and Pico Unicorn have all been converted to MicroPython class-style modules. init() changes to a new class instance. For example this:

import picounicorn

picounicorn.init()

Would become:

from picounicorn import PicoUnicorn

picounicorn = PicoUnicorn()

@Gadgetoid
Copy link
Member Author

Looks like interstate75.py is not included in Pico W builds and should be. The MicroPython module .cmake files need tidying again since they've got quite out of hand.

Gadgetoid and others added 17 commits March 16, 2023 13:25
The result of GET_STR_DATA_LEN should be null terminated, so converting to a std::string and then using .c_str() is both redundant and need
s heap.
MicroPython's GET_STR_DATA_LEN macro returns a const byte array and len, which std::string would copy into heap.

Using string_view lets us wrap the existing const values.
Because `mp_tracked_calloc` does not survive a soft reset but the memory region will, resulting in half-initialised frankenclasses that behave unpredictably.

Using the class pattern fixes this since it's always guaranteed to be initialised when a user instantiates it, and __del__ can handle cleanup.
Borrow heavily from Galactic/Cosmic Unicorn for the PIO/chained-DMA setup.
@Gadgetoid Gadgetoid force-pushed the patch-jpegdec-filename branch 2 times, most recently from a6b5747 to eb0de91 Compare March 16, 2023 13:35
@Gadgetoid Gadgetoid force-pushed the patch-jpegdec-filename branch from eb0de91 to b83bdbf Compare March 16, 2023 15:11
@Gadgetoid Gadgetoid merged commit cb5db1d into main Mar 17, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants